Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AP_DDS: Status Message #28337

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tizianofiorenzani
Copy link

Test

  • Launch the SITL.
  • Open a new terminal and echo the new topic.
  • Verify that only one message is published.
  • Now change the vehicle's mode and verify that a new message has been published.
  • Arm and takeoff, verify that a new message has been published only when armed flag changes.

Test a failsafe:

  • Change the parameter BATT_LOW_VOLT to 11.0
  • Set the parameter SIM_BATT_VOLTAGE to 10.6.
  • Verify that the status topics reports a failsafe of type 2

image

@@ -42,6 +43,7 @@ static constexpr uint16_t DELAY_GEO_POSE_TOPIC_MS = 33;
static constexpr uint16_t DELAY_CLOCK_TOPIC_MS = 10;
static constexpr uint16_t DELAY_GPS_GLOBAL_ORIGIN_TOPIC_MS = 1000;
static constexpr uint16_t DELAY_PING_MS = 500;
static constexpr uint16_t DELAY_STATUS_TOPIC_MS = 500;
Copy link
Collaborator

@Ryanf55 Ryanf55 Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make this event driven instead rather than poll?
This introduces as most 500mS delay on known the vehicle has failsafed which may be a long time.

@Ryanf55
Copy link
Collaborator

Ryanf55 commented Oct 8, 2024

Clean commits before merge.

msg.flying = AP_Notify::flags.flying;
uint8_t fs_iter = 0;
msg.failsafe_size = 0;
if ( AP_Notify::flags.failsafe_radio ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: little whitespace within the brackets

{
// Fill the new message.
msg.vehicle_type = static_cast<uint8_t>(AP::fwversion().vehicle_type);
msg.armed = AP_Notify::flags.armed;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be hal.util->get_soft_armed()

msg.vehicle_type = static_cast<uint8_t>(AP::fwversion().vehicle_type);
msg.armed = AP_Notify::flags.armed;
msg.mode = AP::vehicle()->get_mode();
msg.flying = AP_Notify::flags.flying;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be vehicle->get_likely_flying();

// Fill the new message.
msg.vehicle_type = static_cast<uint8_t>(AP::fwversion().vehicle_type);
msg.armed = AP_Notify::flags.armed;
msg.mode = AP::vehicle()->get_mode();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should do const auto &vehicle = AP::vehicle();

const uint8 APM_AP_BOOTLOADER = 11;
const uint8 APM_BLIMP = 12;
const uint8 APM_HELI = 13;
const uint8 FS_RADIO = 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a separate enum? seems odd to fix FS and APM_xxx

if ( AP_Notify::flags.failsafe_radio ) {
msg.failsafe[fs_iter++] = FS_RADIO;
}
if ( AP_Notify::flags.failsafe_battery ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use this from AP_BattMon

if ( AP_Notify::flags.failsafe_radio ) {
msg.failsafe[fs_iter++] = FS_RADIO;
}
if ( AP_Notify::flags.failsafe_battery ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we could get the battery failsafe state from the battery library.

For RC, EKF and GCS failsafe we probably need to do what you've done here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 🔖 Ready
Development

Successfully merging this pull request may close these issues.

5 participants